Skip to content

test_runner: exclude files with no matching tests from report#64171

Open
Han5991 wants to merge 4 commits into
nodejs:mainfrom
Han5991:fix/test-runner-name-pattern-zero-match-files
Open

test_runner: exclude files with no matching tests from report#64171
Han5991 wants to merge 4 commits into
nodejs:mainfrom
Han5991:fix/test-runner-name-pattern-zero-match-files

Conversation

@Han5991

@Han5991 Han5991 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Under process isolation (the default), a test file whose tests are all filtered out by --test-name-pattern / --test-skip-pattern was reported as a spurious passing entry (✔ file.test.js) and counted in the tests / pass totals. This drops such files from the parent's report so they are neither shown nor counted, matching what --test-isolation=none already does.

Fixes: #64099

Under process isolation, a test file whose tests are all filtered out
by --test-name-pattern or --test-skip-pattern was still reported as a
spurious passing entry and counted in the test totals. Drop such files
from the parent's report, so they are neither shown nor counted. This
matches the existing --test-isolation=none behavior.

Refs: nodejs#64099
Signed-off-by: sangwook <rewq5991@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jun 27, 2026
Han5991 added 3 commits June 27, 2026 22:56
Signed-off-by: sangwook <rewq5991@gmail.com>
Signed-off-by: sangwook <rewq5991@gmail.com>
Comment on lines +288 to +295
const noError = !this.error || this.error.failureType === kSubtestsFailed;
if (!noError) {
return false;
}
if (this.#reportedChildren > 0) {
return true;
}
return this.root.harness.isFilteringByName;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this changes behavior, if I have a file empty.test.mjs, and it has no tests, it won't be included in the report if I'm filtering by name, but will if I'm not. IMO we should either go all the way or none of the way, not partially.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_runner: files without any tests matching --test-name-pattern shouldn't be counted or reported

3 participants